-
Notifications
You must be signed in to change notification settings - Fork 71
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove read_file_to_str helper #245
Conversation
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #245 +/- ##
==========================================
- Coverage 38.08% 38.02% -0.06%
==========================================
Files 55 55
Lines 5099 5086 -13
==========================================
- Hits 1942 1934 -8
+ Misses 3157 3152 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Maksym Pavlenko <pavlenko.maksym@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
#[error("Failed to read file: {0}")] | ||
Io(#[from] std::io::Error), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this type of error always be failed to read a file
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxpv IoError
is defined in L40, I think you can use macro io_error!
to map errors.
The error message in std::io::Error
is not contains any info of filename, for exampe, if the file is not exist, the error would be No such file or directory
without any info to indicate which file or directory is not exist, that is diffcult to debug. Thus, the macro io_error!
is used for load the filename or other details, here is an example:
let finame = "config.json";
File::open(finame)
.await
.map_err(io_error!(e, "failed to open {}: ", finame))?;
and the error would be failed to open config.json error: No such file or directory
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really like the requirement to map errors and use macros everywhere, like
map_err(io_error!
. I feel like anyhow.Context
would be a better fit here. Will investigate this more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
However, anyhow error is not easy to downcast if you need check the exact type of error
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where exactly we need to downcast?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where exactly we need to downcast?
When you want to ignore some specific error.
#[error("Failed to read file: {0}")] | ||
Io(#[from] std::io::Error), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mxpv IoError
is defined in L40, I think you can use macro io_error!
to map errors.
The error message in std::io::Error
is not contains any info of filename, for exampe, if the file is not exist, the error would be No such file or directory
without any info to indicate which file or directory is not exist, that is diffcult to debug. Thus, the macro io_error!
is used for load the filename or other details, here is an example:
let finame = "config.json";
File::open(finame)
.await
.map_err(io_error!(e, "failed to open {}: ", finame))?;
and the error would be failed to open config.json error: No such file or directory
Use
fs::read_to_string
from stdlib/tokio instead.